issue 1166 Update test to use SQLAlchemy 2.0 compatible select statement #1167
Conversation
loicdiridollou
left a comment
loicdiridollou
left a comment
There was a problem hiding this comment.
One small change to make sure we are testing the right thing.
tests/test_io.py
Outdated
| pd.read_sql( | ||
| session.query(Temp.quantity).statement, session.connection() | ||
| ) | ||
| stmt = sqlalchemy.select(Temp.quantity) |
There was a problem hiding this comment.
Here you are modifying the type of stmt so you are not really testing what used to be tested. What is needed is to update the stubs so that this test passes.
Without changing this test, what needs to be done is to update a pyi file.
For that I would recommend you look into what is the new type of stmt with the update of sqlalchemy, you will see that stmt has now an extra possible type UpdateBase and what how that impacts the stubs in pandas-stubs/io/sql.pyi. There you see that the Type Alias _SQLStatement is too restrictive and you need to allow UpdateBase in that TypeAlias too.
Feel free to let me know if something is unclear.
|
@loicdiridollou I reverted my previous commits and added extra possible type UpdateBase to _SQLStatement, is that ok? |
pandas-stubs/io/sql.pyi
Outdated
| | sqlalchemy.sql.expression.TextClause | ||
| | sqlalchemy.sql.Select | ||
| | FromStatement | ||
| | UpdateBase |
There was a problem hiding this comment.
Instead of importing UpdateBase, use the pattern as in lines 33 and 34, and place sqlalchemy.sql.expression.UpdateBase here
|
@Dr-Irv Applied the change |
Dr-Irv
left a comment
Dr-Irv
left a comment
There was a problem hiding this comment.
while you were working on this, we put a constraint in pyproject.toml on sqlalchemy so that our CI would pass.
Can you change this line in that file:
SQLAlchemy = ">=2.0.12,<2.0.39"
to
SQLAlchemy = ">=2.0.39"
Co-authored-by: Irv Lustig <[email protected]>
3 participants
This PR resolves #1166 by update test to use SQLAlchemy 2.0 compatible select statement